Skip to content

Add buffer storage policies with mandatory logging#2

Closed
PhysShell wants to merge 20 commits into
mainfrom
claude/untitled-tyoeaq
Closed

Add buffer storage policies with mandatory logging#2
PhysShell wants to merge 20 commits into
mainfrom
claude/untitled-tyoeaq

Conversation

@PhysShell

Copy link
Copy Markdown
Owner

Introduce explicit buffer storage policies (the stackalloc/scratch/pool/
native/inline line) as owned resources with a stated storage intent, plus
the three logging surfaces that keep the choice honest.

Language & front-end:

  • Buffer.(size, name = value, ...) intent expressions; policy { ... }
    declaration blocks (lexer DOT token + policy keyword, parser, AST).
  • buffers.py centralizes mode/policy resolution, defaults, and bound validation.

Checker:

  • Buffers are ownership-checked like any resource (release-exactly-once,
    borrows, escape). Stack-backed modes (stack/scratch/inline) cannot escape.
  • New diagnostics: OWN015 (escape via return), OWN016 (escape via consume),
    OWN019 (inline capacity too large), OWN021 (dynamic stack needs a bound),
    OWN023 (scratch forbids fallback but size may exceed inline).

Codegen:

  • Real C# per mode: stackalloc, stack-first/ArrayPool-fallback scratch with a
    try/finally Return, pooled, NativeMemory (unsafe), inline, plus clear-on-release.
  • OwnTrace (text) and OwnCounters (metrics) runtime hooks under
    [Conditional("OWNSHARP_TRACE"/"OWNSHARP_COUNTERS")], emitted alongside any
    module that uses buffers.

Logging surfaces:

  • report.py + report CLI command: compile-time buffer report to stdout and
    .ownreport.json (mode, inline limit, fallback, escape policy, branches, checks).
  • Runtime trace + counters (above).

Tests & docs:

  • 18 new analysis cases, buffer codegen + report smoke; suite stays green.
  • README section documenting the policies, lowering, logging, and example files.

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7

Introduce explicit buffer storage policies (the stackalloc/scratch/pool/
native/inline line) as owned resources with a stated storage intent, plus
the three logging surfaces that keep the choice honest.

Language & front-end:
- Buffer.<mode>(size, name = value, ...) intent expressions; `policy { ... }`
  declaration blocks (lexer DOT token + `policy` keyword, parser, AST).
- buffers.py centralizes mode/policy resolution, defaults, and bound validation.

Checker:
- Buffers are ownership-checked like any resource (release-exactly-once,
  borrows, escape). Stack-backed modes (stack/scratch/inline) cannot escape.
- New diagnostics: OWN015 (escape via return), OWN016 (escape via consume),
  OWN019 (inline capacity too large), OWN021 (dynamic stack needs a bound),
  OWN023 (scratch forbids fallback but size may exceed inline).

Codegen:
- Real C# per mode: stackalloc, stack-first/ArrayPool-fallback scratch with a
  try/finally Return, pooled, NativeMemory (unsafe), inline, plus clear-on-release.
- OwnTrace (text) and OwnCounters (metrics) runtime hooks under
  [Conditional("OWNSHARP_TRACE"/"OWNSHARP_COUNTERS")], emitted alongside any
  module that uses buffers.

Logging surfaces:
- report.py + `report` CLI command: compile-time buffer report to stdout and
  .ownreport.json (mode, inline limit, fallback, escape policy, branches, checks).
- Runtime trace + counters (above).

Tests & docs:
- 18 new analysis cases, buffer codegen + report smoke; suite stays green.
- README section documenting the policies, lowering, logging, and example files.

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57ae2ff343

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py
Comment thread codegen.py Outdated
Comment thread buffers.py Outdated
Address Codex review on PR #2:

- P1: an escaping pooled/native buffer (returned or passed to a `consume`
  call) no longer runs its finally cleanup. Ownership transfers to the new
  owner, so returning the array to ArrayPool / freeing native memory here
  was a use-after-return / double-free. Codegen now detects the escape (no
  matching `release`) and suppresses the cleanup.

- P2: a constant scratch smaller than its inline limit
  (e.g. scratch(64, inline = 1024, fallback = forbidden)) now reserves the
  inline capacity but slices the span to the requested length, instead of
  exposing the full reservation.

- P2: BufferInfo.branches() reports a fallback-forbidden scratch as
  stack-only, so the compile-time report no longer advertises an ArrayPool
  branch that cannot occur.

Adds escape_and_length_smoke regression guards; suite stays green.

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5d52543e3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py
if info.counters:
pre.append(f"OwnCounters.PoolFallback({size});")
pre.append(f"byte[] {name}_array = ArrayPool<byte>.Shared.Rent({size});")
pre.append(f"Span<byte> {name} = {name}_array.AsSpan(0, {size});")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the rented array when pooled buffers escape

With an escaping pooled buffer, e.g. fn f(n:int)->Buffer { let b = Buffer.pooled(n); return b; } or a consume call, codegen suppresses the ArrayPool.Return, but the only value named b is this Span<byte> while the rented byte[] remains a local. The caller/callee receives no handle it can return to the pool (and the return type is still Buffer), so a clean program either emits uncompilable C# or leaks the rent; the escaping representation needs to carry the array/length or be rejected.

Useful? React with 👍 / 👎.

Comment thread codegen.py Outdated
# no matching `release` => the buffer escapes (it was returned or
# consumed; the checker already proved this is the only clean
# reason). Ownership transferred, so cleanup must NOT run here.
escapes = j is None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not treat non-top-level buffer releases as escapes

When the matching release is not a following top-level release b—for example let b = Buffer.pooled(n); if (c) { release b; } else { release b; }, which the analyzer accepts—j is None and this path marks the buffer as escaped. The generated body then drops the pool/native cleanup and emits the nested releases through generic b.Dispose() on a Span<byte>, so branchy clean buffer code leaks or fails to compile.

Useful? React with 👍 / 👎.

Comment thread report.py
lo, hi = spans[id(fn)]
fn_diags = [d for d in diags if lo <= d.line < hi]
for name, intent in _walk_buffers(fn.body):
info, _ = resolve_buffer(intent, policies)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip unresolved buffer modes when building reports

For an invalid-but-diagnosable input such as Buffer.bogus(n), build_cfg records OWN030 and avoids resolving the mode, but report calls resolve_buffer again unconditionally here. That raises ValueError inside python -m ownlang report instead of producing the report/diagnostic for the typo, so the new reporting surface crashes on malformed buffer intents.

Useful? React with 👍 / 👎.

claude added 2 commits June 13, 2026 23:31
Follow-up to the PR #2 review: suppressing the cleanup on an escaping pooled/
native buffer stopped the double-free but left the other horn — the escaping
value is the Span<byte> while the rented byte[] stays a dead local, so the
caller gets no handle to Return and the `Buffer` return type isn't real C#.
The result was uncompilable-or-leaky.

Per the decision that the checker is the deliverable and codegen should only
prove the model lowers to real .NET (not grow a faithful movable-buffer
representation), reject the escape in the checker instead of faking it:

- New OWN017: a movable (pooled/native) buffer that escapes via return or a
  consume call is rejected. The ownership model still treats pooled/native as
  movable; this is an honest codegen-capability boundary (like OWN020), so the
  PoC never emits broken C#. Faithful movable lowering (byte[]-handle or an
  IMemoryOwner<byte> wrapper) is left as roadmap. Locally-released pooled/native
  buffers still work fully (real ArrayPool.Return / NativeMemory.Free).

- buffer_scratch_program.cs.txt: a runnable golden whose parse() and
  OwnTrace/OwnCounters are emitted verbatim, with a hand-written host + Main,
  proving the buffer model lowers to real, runnable ArrayPool .NET. A drift
  guard in the suite keeps it in sync with the emitter.

- Tests: pooled/native escape now assert OWN017; added a local-pooled clean
  case. README corrected (movable escape rejected in PoC; runnable golden;
  straight-line buffer codegen limitation documented). Suite green (62 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
Two follow-up review fixes:

- Branchy buffer release is no longer mistaken for an escape. After the OWN017
  change, any buffer reaching codegen in a clean program is non-escaping, so a
  buffer whose `release` sits inside if/borrow branches (e.g.
  `let b = Buffer.pooled(n); if (c) { release b; } else { release b; }`, which
  the analyzer accepts) previously dropped its cleanup and emitted the nested
  releases as a generic `b.Dispose()` on a Span<byte> — a leak / uncompilable.
  Codegen now detects a nested release and emits the buffer's real cleanup
  (pool Return / native Free / clear) at each release site, no finally hoist —
  the same trade-off already used for branchy ordinary resources. A genuine
  "no release anywhere" is now a hard CodegenError (the checker rejects it
  first via OWN001/OWN015/OWN016/OWN017).

- build_report skips a malformed buffer mode (e.g. Buffer.bogus(n)) instead of
  calling resolve_buffer and throwing ValueError; the checker's OWN030 stands.
  `report` also now surfaces diagnostics instead of swallowing them, and exits
  non-zero on errors.

Tests: branchy-release clean case + branchy_and_malformed_smoke; README note
corrected (branchy release is handled, not a limitation). Suite green (63 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a94513b02

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py Outdated
if j is not None:
# straight-line: release b at this level -> exception-safe try/finally,
# then continue with whatever follows the release.
lines = self._wrap_buffer(pre, rest[:j], fin, ind, native)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle non-LIFO buffer releases in codegen

With clean code such as let a = Buffer.pooled(n); let b = Buffer.pooled(n); release a; release b;, the checker accepts both buffers, but this call wraps only the statements before a's release and recursively emits b without seeing its later release. _emit_buffer then raises CodegenError for a valid program, so emit fails whenever overlapping buffer lifetimes are released in non-LIFO order.

Useful? React with 👍 / 👎.

Comment thread buffers.py Outdated
Comment on lines +227 to +228
else:
fallback_pool = True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject unknown scratch fallback values

When fallback is present but misspelled or unsupported, for example fallback = forbiden in source or a policy, fb is neither None nor "forbidden", so this branch silently enables ArrayPool fallback. That defeats the explicit storage-policy guarantee: a typo in a setting intended to forbid heap allocation compiles and rents from the pool instead of producing a diagnostic.

Useful? React with 👍 / 👎.

Comment thread report.py Outdated

# Diagnostics that, if present for a given buffer, mean a specific check failed.
_CHECK_CODES = {
"noEscape": {"OWN015", "OWN016"},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mark OWN017 as a noEscape failure

Escaping pooled/native buffers are now rejected with OWN017, but the report's noEscape check only looks for OWN015 and OWN016. For a rejected program such as let b = Buffer.pooled(n); return b;, report still emits checks.noEscape: true, so the machine-readable logging surface contradicts the checker diagnostic and can mislead CI/review tooling.

Useful? React with 👍 / 👎.

Three follow-up review fixes:

- Overlapping buffer lifetimes released in non-LIFO order no longer crash
  codegen. The previous "buffer owns the rest of the block, nested try/finally"
  model assumed LIFO; `let a=Buffer.pooled(n); let b=Buffer.pooled(n);
  release a; release b;` (which the checker accepts) raised a spurious
  CodegenError. Codegen now uses the exception-safe try/finally only when a
  buffer nests cleanly (straight-line release here, no other buffer acquired in
  its body); otherwise it uses inline-release (prelude at the let, real cleanup
  at each release site), which also subsumes the branchy case. Native buffers
  are emitted with a method-level `unsafe` instead of per-buffer unsafe blocks,
  so they follow the same lifetime shapes.

- A misspelled/unsupported scratch fallback (e.g. `fallback = forbiden`, in
  source or a policy) is now diagnosed (OWN030) and fails safe, instead of
  silently falling through to enable the ArrayPool heap fallback — which would
  quietly break the explicit storage guarantee.

- The report's noEscape check now includes OWN017, so the machine-readable
  surface no longer claims noEscape:true for a buffer the checker rejected for
  escaping.

Tests: non-LIFO + LIFO overlap cases, misspelled-fallback case, and smoke
guards for FIFO codegen / fallback typo / report-noEscape agreement. README
updated. Suite green (66 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ae71b862b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py Outdated
Comment on lines +158 to +163
j = self._find_release(rest, 0, name)
if j is not None and not _fn_has_buffer(rest[:j]):
out.extend(self._emit_buffer_scoped(name, st.rhs, rest[:j], ind))
i += 1 + j + 1 # consume the let, its body, and its release
continue
if not _has_release(rest, name):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle buffer releases through moved aliases

When a buffer is moved and the new owner is released, e.g. let a = Buffer.pooled(n); let b = move a; release b;, the checker accepts the program but emit fails because this scan only looks for release a after the original declaration and treats the buffer as unreleased. Buffers are otherwise allowed to move within a function, so codegen needs to track the current owning alias (and attach cleanup to that alias) rather than requiring the declaration name to be released.

Useful? React with 👍 / 👎.

Comment thread parser.py
Comment on lines +304 to +306
ns = self.eat(Tok.IDENT) # namespace, conventionally "Buffer"
self.eat(Tok.DOT)
mode = self.eat(Tok.IDENT).text

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-Buffer namespaces for buffer intents

This parses any dotted constructor with a known mode as a buffer intent because the namespace token is discarded; for example Foo.stack(n, max = 1024) checks clean and lowers to a real stack buffer. Since the documented syntax is Buffer.<mode>(...), mistyping Buffer or using an unrelated identifier should be diagnosed instead of silently changing the program into a buffer allocation.

Useful? React with 👍 / 👎.

Comment thread report.py Outdated
Comment on lines +67 to +68
mine = [d for d in fn_diags if f"'{name}'" in d.message]
mine_codes = {d.code for d in mine}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mark checks failed after buffer aliases escape

For an escaping moved buffer such as let a = Buffer.stack(n, max = 100); let b = move a; return b;, analysis reports OWN015 on 'b', but the report entry is for the declared buffer 'a' and this name-based filter misses the diagnostic, so .ownreport.json incorrectly says checks.noEscape is true. The report should associate diagnostics through buffer ownership aliases or symbol identity rather than only matching the original let-name in the message.

Useful? React with 👍 / 👎.

Three follow-up review fixes around buffer aliasing:

- A buffer released through a moved alias now lowers correctly. The codegen
  release-scan only matched the declaration name, so
  `let a = Buffer.pooled(n); let b = move a; release b;` (accepted by the
  checker) raised a spurious CodegenError. Codegen now follows the move chain:
  the release-detection guard is alias-aware, and a `move` transfers the pending
  cleanup to the new owner so `release <new>` returns/frees the original backing.

- A wrong namespace is diagnosed. The namespace token was discarded, so
  `Foo.stack(n, max = 1024)` silently became a stack buffer. The documented
  syntax is `Buffer.<mode>(...)`; the AST now carries the namespace and the
  checker rejects anything but `Buffer` with OWN030.

- The report attributes diagnostics through move-aliases. For
  `let a = ...; let b = move a; return b;` the escape is reported on 'b', but
  the report entry is for 'a'; the name-only filter missed it and claimed
  noEscape:true. build_report now matches diagnostics against the buffer's
  move-alias set (and skips bad-namespace intents like it skips bad modes).

Tests: move-release clean case, bad-namespace case, moved-escape case, plus
codegen/report alias smoke guards. README updated. Suite green (69 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70652fb918

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread buffers.py Outdated
Comment on lines +213 to +216
fb = _as_ident(opts.get("fallback")) if "fallback" in opts \
else _as_ident_or_none(base.get("fallback"))
if fb is None:
fb = "pool" # scratch defaults to a pool fallback

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-identifier fallback values

When a scratch buffer spells fallback with a non-identifier value such as fallback = 0, _as_ident(...) returns None and this branch treats it exactly like an omitted option, defaulting to pool. That accepts an invalid storage policy with no OWN030 and emits an ArrayPool fallback, so a malformed attempt to forbid/override fallback can silently heap-allocate instead of failing safe like the string-typo path does.

Useful? React with 👍 / 👎.

A scratch fallback spelled with a non-identifier value (e.g. `fallback = 0`,
in source or a policy) made _as_ident return None, which was indistinguishable
from an omitted option and defaulted to enabling the ArrayPool heap fallback —
so a malformed attempt to forbid/override the fallback silently heap-allocated
instead of failing safe like the string-typo path.

resolve() now distinguishes "absent" (default pool) from "present but
malformed". A present-but-invalid value — string typo or non-identifier — is
diagnosed (OWN030) and fails safe (forbidden, never silently pooled), for both
the inline option and a policy-supplied value.

Tests: non-identifier fallback analysis case + a resolve smoke asserting OWN030
and fallback_pool == False. Suite green (70 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9254b7288

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py Outdated
Comment on lines +71 to +72
if _fn_has_buffer(self.fn.body):
return False

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve finally cleanup for ordinary resources

When any buffer appears in an otherwise straight-line function, this forces the whole method off _emit_simple, so normal acquire/release pairs inside the buffer lifetime are emitted inline inside the buffer's try body. For example, let b = Buffer.scratch(n); let c = acquire Conn(); Work(b); release c; release b; generates c.close() before the buffer finally, so an exception from Work returns the buffer but leaks Conn; the pre-existing simple path would have put Conn in its own finally.

Useful? React with 👍 / 👎.

Comment thread codegen.py Outdated
Comment on lines +272 to +273
pre.append(f"byte* {name} = (byte*)System.Runtime.InteropServices."
f"NativeMemory.Alloc((nuint){size});")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject negative native buffer sizes before casting

For Buffer.native(n), n is an int, and this lowering casts it directly to nuint; when a caller passes a negative size, the cast wraps to a huge unsigned value and requests an enormous unmanaged allocation instead of rejecting the input. The stack path already uses a (uint)size > limit guard for dynamic sizes, so native buffers need a non-negative/range guard before NativeMemory.Alloc (and before using the same size for clear).

Useful? React with 👍 / 👎.

Comment thread buffers.py Outdated
Comment on lines +300 to +305
for v in (_as_ident(opt_expr) if opt_expr is not None else None, policy_val):
if isinstance(v, str):
return v not in ("off", "none", "false")
if opt_expr is not None:
return _truthy(opt_expr)
return default

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Honor policy trace=false

A policy value trace = false is parsed as the boolean False, but _trace_flag only acts on string policy values and then falls through to the default when there is no inline option. As a result policy P { trace = false; } still reports trace: true and emits OwnTrace calls, unlike inline trace = false, so policy-level logging settings are ignored.

Useful? React with 👍 / 👎.

… trace

Three follow-up review fixes:

- Ordinary resources keep their own finally when a buffer is present. A buffer
  no longer forces the whole function onto the inline path; a straight-line
  function uses the try/finally hoist, extended to nest buffers AND ordinary
  resources, each in its own finally (LIFO release order). Previously
  `let b = Buffer.scratch(n); let c = acquire Conn(); Work(b); release c;
  release b;` emitted c.close() inside the buffer's try body, so an exception
  from Work returned the buffer but leaked Conn. Now Conn gets its own nested
  finally. (Branchy functions still use inline-release, unchanged.)

- Buffer.native(n) with a dynamic size guards against a negative request
  (`if (n < 0) throw ...`) before NativeMemory.Alloc, so a negative int can't
  wrap to a huge nuint and request an enormous unmanaged allocation. Constant
  sizes (never negative in this grammar) emit no guard.

- A policy `trace = false` is honored. _trace_flag ignored boolean policy
  values and fell through to the default (true), so `policy P { trace = false; }`
  still emitted OwnTrace and reported trace:true. It now respects bool/string
  policy values, matching inline `trace = false`.

Tests: nesting_native_trace_smoke covers all three. README updated. Golden
unchanged. Suite green (70 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ee9c74c35

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py Outdated
# the pending cleanup so a later `release <new>` returns/frees the
# original backing, and alias the C# local.
if st.rhs.var in self.buffer_cleanup:
self.buffer_cleanup[st.name] = self.buffer_cleanup.pop(st.rhs.var)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep buffer cleanup available in sibling branches

When a move is emitted while traversing one branch, this destructively removes the original buffer's cleanup from the shared codegen state. For a checker-clean function such as let a = Buffer.pooled(n); if (c) { let b = move a; release b; } else { release a; }, the then branch pops a before the else branch is emitted, so the else release falls back to a.Dispose() instead of returning a_array to the pool. The branch traversal needs branch-local state or non-destructive aliasing for buffer cleanups.

Useful? React with 👍 / 👎.

Comment thread codegen.py Outdated
if info.trace:
pre.append(f'OwnTrace.PooledSelected("{fn}", "{name}", {size});')
if info.counters:
pre.append(f"OwnCounters.PoolFallback({size});")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not count pooled allocations as scratch fallbacks

For Buffer.pooled(...), this emits OwnCounters.PoolFallback, which increments the ScratchPoolFallbacks/ScratchPoolBytesRented counters. Any program that uses ordinary pooled buffers alongside scratch buffers will therefore report normal pool allocations as scratch fallback misses, corrupting the runtime metric that is supposed to answer whether scratch requests actually hit the stack.

Useful? React with 👍 / 👎.

Comment thread codegen.py Outdated
Comment on lines +108 to +109
elif isinstance(st, A.Let) and isinstance(st.rhs, A.BufferIntent):
scopes.append(self._buffer_lowering(st.name, st.rhs))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve statement order before buffer allocation

Adding every buffer let to scopes makes simple-mode codegen emit all buffer preludes before every non-scope statement in the function. A checker-clean body like let n = 64; let b = Buffer.pooled(n); release b; lowers to Rent(n) and trace calls before var n = 64;, and setup calls placed before a buffer are likewise moved after the allocation inside the try body. Buffer preludes should remain after earlier plain statements they can depend on, or simple mode should bail out for those shapes.

Useful? React with 👍 / 👎.

…de ordering

Three follow-up review fixes:

- A `move` no longer destroys the original buffer's cleanup for sibling
  branches. The move handler popped buffer_cleanup[a] when emitting one branch,
  so for `if (c) { let b = move a; release b; } else { release a; }` the else
  branch fell back to a.Dispose() on a Span instead of returning a_array. It now
  copies (non-destructive) the cleanup to the new alias; keeping the original
  registered is safe because releasing it after a move is use-after-move
  (rejected upstream), so both branches return to the pool.

- The Scratch.* counters (StackHit / PoolFallback / Release) are emitted only
  for scratch buffers. Pooled emitted PoolFallback, and pooled/native/stack
  emitted Release/StackHit, polluting the metric that answers "do scratch
  requests hit the stack?". Non-scratch buffers are still traced, just not
  counted.

- Simple-mode codegen emits in source order. It previously hoisted every
  buffer/acquire prelude above the whole body, so `let n = 64; let b =
  Buffer.pooled(n); ...` emitted Rent(n) before `var n = 64;` (and setup calls
  landed after the allocation). The hoist is now recursive: leading plain
  statements stay before a scope's prelude, the scope's try wraps the rest, and
  releases still run LIFO.

Tests: sibling-move analysis case + ordering_counters_smoke covering all three.
README/golden unchanged. Suite green (71 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 996e73027e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py Outdated
Comment on lines +111 to +115
rest = self._drop_release(stmts[i + 1:], st.name)
if fin:
out.append(f"{base}try")
out.append(f"{base}{{")
out.extend(self._emit_hoist(rest, base + ind))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge End hoisted buffer scopes at their release

In straight-line functions this drops the first release but then emits every following statement inside the same try. For let a = Buffer.pooled(n); release a; let b = Buffer.pooled(n); ..., the generated C# rents b before returning/clearing a, so the source release point is ignored; this extends pool/native lifetimes and delays clear_on_release, skewing memory use and logging. Split at the release and emit the tail after the finally.

Useful? React with 👍 / 👎.

Comment thread buffers.py Outdated
Comment on lines +169 to +172
pol_name = _as_ident(pol_expr)
if pol_name in policies:
base = dict(policies[pol_name].settings)
elif pol_name is not None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-identifier policy references

When policy = is present but not an identifier, _as_ident() returns None and this path emits no diagnostic, so Buffer.scratch(n, policy = 0) silently falls back to default inline/fallback/clear settings. That can bypass an intended policy such as clear_on_release = true without the OWN030 promised for unknown policy values; diagnose non-identifier policy expressions the same way fallback validation does.

Useful? React with 👍 / 👎.

Comment thread codegen.py
pre.append(f"byte[]? {name}_rented = null;")
pre.append(f"Span<byte> {name}_backing = stackalloc byte[{L}];")
pre.append(f"Span<byte> {name};")
pre.append(f"if ({size} <= {L})")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard negative scratch sizes before logging hits

For Buffer.scratch(n) with a negative runtime n, this condition takes the stack branch because n <= inlineLimit, emits ScratchSelected/StackHit, and then the [..n] slice throws before the generated try/finally can increment Release. The stack/native paths already reject negative sizes before logging; scratch should use the same unsigned check or an explicit n < 0 guard so invalid inputs do not corrupt the runtime counters/trace.

Useful? React with 👍 / 👎.

…olicy ref

Three follow-up review fixes:

- Simple-mode now splits each scope at its `release`: statements during the
  lifetime go in the try, statements after the release are emitted after the
  finally. Previously every following statement was wrapped in the same try, so
  `let a = Buffer.pooled(n); release a; let b = Buffer.pooled(n); ...` rented b
  before returning a, extending pool/native lifetimes and delaying
  clear_on_release. Disjoint sequential resources now stay disjoint; overlapping
  ones still nest LIFO.

- A non-identifier policy reference (e.g. `policy = 0`) is now diagnosed
  (OWN030), instead of silently using default settings and bypassing an intended
  policy such as clear_on_release = true.

- A dynamic scratch size is guarded against a negative request before any
  trace/counter runs. Previously `scratch(n)` with negative n took the stack arm
  (n <= limit), logged ScratchSelected/StackHit, then the [..n] slice threw
  before the try could balance Release — corrupting the counters/trace. Matches
  the existing stack/native guards.

Tests: disjoint-sequential + bad-policy analysis cases, plus smoke guards for
disjoint ordering and the scratch negative guard. README updated. Golden
unchanged. Suite green (73 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce7db105b7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codegen.py
Comment on lines +367 to +368
if st.rhs.var in self.buffer_cleanup:
self.buffer_cleanup[st.name] = self.buffer_cleanup[st.rhs.var]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Scope moved-buffer cleanup aliases

When a buffer is moved inside a nested block, this records the moved-to name in the function-wide buffer_cleanup map and never removes it when that source scope ends. In a clean program such as if (c) { let b = move a; release b; } else { release a; } let b = acquire Conn(); release b;, the resolver allows the later unrelated b, but codegen still treats release b as the stale buffer alias and emits a second ArrayPool<byte>.Shared.Return(a_array) instead of b.close(), double-returning the array and leaking the resource. The cleanup aliases need to be scoped or cleared on redeclaration/block exit.

Useful? React with 👍 / 👎.

Comment thread buffers.py
Comment on lines +184 to +187
if name in opts:
v = _as_int(opts[name])
if v is not None:
return v

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-integer buffer bounds

When an inline/inline_bytes option is present but not an integer, this helper silently falls through to policy/default values. For example, Buffer.scratch(n, inline = bogus) currently checks clean and emits/logs a 1024-byte inline limit, so a misspelled or non-literal stack budget silently changes the storage policy instead of failing safe. Please diagnose present non-integer bounds rather than defaulting.

Useful? React with 👍 / 👎.

{
public static void parse(int size)
{
byte[]? tmp_rented = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Regenerate scratch golden with the size guard

The checked-in runnable golden is advertised above as the current emitter's verbatim parse body, but the current emitter now writes a negative-size guard before the rent/backing declarations. In this file, calling parse(-1) reaches OwnTrace.ScratchSelected/OwnCounters.StackHit() and then throws on tmp_backing[..size] before the try/finally, corrupting the demo counters; regenerate the golden so it includes the guard.

Useful? React with 👍 / 👎.

Three follow-up review fixes:

- Buffer cleanup aliases no longer leak across scopes. A `move` registered the
  moved-to name in the function-wide buffer_cleanup map and never cleared it, so
  after `if (c) { let b = move a; release b; } ...` a later unrelated
  `let b = acquire Conn(); release b;` emitted a second
  ArrayPool.Return(a_array) (double-return + Conn leak) instead of b.close().
  A (re)declaration of a name now clears any stale cleanup alias for it
  (in _stmt_inline and _buffer_lowering), so the later b is closed correctly and
  a_array is returned exactly once per branch.

- Non-integer buffer bounds are diagnosed. `inline = bogus` / `inline_bytes` /
  `max = bogus` / policy `max_bytes` that are present but not integers now emit
  OWN030 instead of silently falling through to the default and changing the
  storage policy.

- The runnable scratch golden is regenerated to include the negative-size guard
  the emitter now writes before the rent/backing declarations, so parse(-1)
  throws before any trace/counter instead of corrupting the demo metrics. The
  drift guard now also checks the guard line and its ordering.

Tests: alias-redeclaration, bad-inline-bound and bad-max-bound analysis cases,
plus codegen smoke for the redeclaration and a tightened golden drift guard.
Suite green (76 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 777428af13

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread buffers.py Outdated
inline_bytes = mx_val

elif mode == BufferMode.SCRATCH:
inline_bytes = opt_int("inline", opt_int("inline_bytes", DEFAULT_INLINE_BYTES))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect inline overrides before validating policy defaults

When a scratch buffer supplies an inline inline = ... override, the policy inline_bytes default should be irrelevant, but this call evaluates opt_int("inline_bytes", ...) before opt_int("inline", ...) runs. In a case like policy P { inline_bytes = bogus; } plus Buffer.scratch(n, policy = P, inline = 128), the checker still emits OWN030 for the ignored policy value, contradicting the documented rule that inline options override policy defaults and rejecting a program whose effective buffer configuration is valid.

Useful? React with 👍 / 👎.

@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ad85bd82c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread parser.py
key = self.cur.text
self.pos += 1
self.eat(Tok.EQ)
options[key] = self.parse_atom()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject duplicate buffer options

When a buffer intent repeats a named option, this assignment silently overwrites the earlier value, so Buffer.scratch(n, fallback = forbidden, fallback = pool) is accepted and lowered/reported as having an ArrayPool fallback instead of diagnosing the conflicting storage promise. That defeats the mandatory storage-policy checks for copy/paste mistakes on sensitive options such as fallback, clear, or policy; please reject duplicate option keys rather than letting the last one win.

Useful? React with 👍 / 👎.

A repeated named option (e.g. Buffer.scratch(n, fallback = forbidden,
fallback = pool)) silently overwrote the earlier value in the options dict, so a
conflicting storage promise was accepted and lowered/reported as the last value
won — defeating the storage-policy checks on copy/paste mistakes. The same held
for repeated keys in a policy block.

The parser now records duplicate keys (before the dict collapses them) on the
AST; resolve rejects duplicate buffer options and validate_policies rejects
duplicate policy settings, both with OWN030.

Tests: duplicate-option and duplicate-policy-setting analysis cases. README
updated. Suite green (93 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccb9cb5264

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread cfg.py Outdated
"OWN018",
f"buffer size '{rhs.size.name}' must be an integer, not "
f"{ssym.kind.name.lower()}", rhs.size.line))
elif ssym is not None and ssym.type_name not in (None, "int"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-int temporaries used as buffer sizes

When the size expression is a plain symbol whose type_name is None, this accepts it as if it were an int. A clean program can copy a borrow into a temporary, e.g. fn f(x: &Buffer){ let n = x; let b = Buffer.pooled(n); release b; }, and the checker emits no diagnostics, but codegen then passes a ReadOnlySpan<byte> to OwnTrace.PooledSelected, ArrayPool<byte>.Shared.Rent(n), and AsSpan(0, n), producing uncompilable C#. Unknown-type plain values should not satisfy the integer-size requirement.

Useful? React with 👍 / 👎.

Comment thread codegen.py
out.extend(self._emit_hoist(body, base))
# statements after this scope's release are siblings, emitted
# after its finally so its lifetime ends at the source release.
out.extend(self._emit_hoist(tail, base))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve locals declared before a hoisted release

Emitting the post-release tail after closing the generated try/finally puts any locals declared between the acquire and the source release out of C# scope. For example, let b = Buffer.pooled(n); let x = 1; release b; let y = x; passes the checker, but codegen declares x inside the try and emits var y = x; after the finally, so the generated C# does not compile. The lowering needs to avoid wrapping declarations that are used after the release, or lift them outside the generated block.

Useful? React with 👍 / 👎.

…ase in scope

Two review fixes:

- A plain temporary of unknown type (type_name None) was accepted as a buffer
  size. A copy of a borrow, e.g. `let n = x;` for `x: &Buffer`, made n a plain
  value with no known type, so `Buffer.pooled(n)` checked clean and lowered to
  Rent(span)/AsSpan(0, span). The size now must be exactly `int`; an
  unknown-typed plain is OWN018.

- A plain local declared between a buffer/resource acquire and its release was
  emitted inside the hoisted try, so a later `let y = x;` after the finally
  referenced an out-of-scope x and did not compile. _is_simple no longer hoists
  when a scope's body declares a plain local (and the inline path likewise uses
  inline-release rather than a wrapping try), so the local stays at function
  scope. Bodies without such locals (the golden, mixed LIFO, disjoint) still get
  the exception-safe hoist.

Tests: borrow-temp-size and local-after-release analysis cases + a smoke
asserting the buffer is not hoisted and x precedes y. README updated. Suite
green (95 cases).

https://claude.ai/code/session_015GuQLyKMRZ23JsXobhn8E7
@PhysShell

Copy link
Copy Markdown
Owner Author

@codex plz check new changes

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

PhysShell pushed a commit that referenced this pull request Jun 14, 2026
PR #2 was authored with package-style relative imports (from . import ...)
but committed as flat files, so it never actually ran. Re-home it onto the
package layout established by PR #1: modules into ownlang/, buffers.py and
report.py alongside, the test runner + its buffer_scratch_program.cs.txt
fixture into tests/, example programs into examples/. Merge .gitignore and
splice PR #1's README quickstart fix into PR #2's README. No logic changes.
PhysShell pushed a commit that referenced this pull request Jun 14, 2026
PR #2's rewritten Return handling set the owner to ESCAPED without checking
for a live loan, so `borrow b as s { return b; }` returned a resource out
from under an outstanding borrow with no diagnostic. PR #1 caught this on the
original analysis; re-apply it, adapted to PR #2's buffer-aware Return block:
returning an owner needs Own permission, so a live loan is OWN007 (as for move).

Copy link
Copy Markdown
Owner Author

Superseded by #3, which landed all three PRs as one linear integration on main (merged in 01da03d). This buffer-policy feature is included there, ported into the ownlang/ package (it was authored with package imports but committed flat, so it never actually ran — re-homed, its 95-case suite is green). Two fixes were applied on top: the codegen hoist was dropping owned-parameter releases (a leak, caught by the new property fuzzer), and returning a resource while borrowed is now flagged OWN007. Closing.


Generated by Claude Code

@PhysShell PhysShell closed this Jun 14, 2026
PhysShell pushed a commit that referenced this pull request Jun 14, 2026
The genuinely-new analysis on top of the ownership core: it reasons about region
escape (the WPF 'zombie ViewModel promoted to App lifetime' theorem), not just
release within a scope.

Language:
- lifetime declarations with a strict order: 'lifetime Window < App;'
  (transitive; cycles -> OWN036, undefined refs -> OWN030, redef -> OWN031).
- lifetime annotations on the object a function sets up and on its service
  params: 'fn VM(bus: EventBus lifetime App) lifetime ViewModel'.
- a strong-capture statement: 'subscribe self to SOURCE;'
  ('self'/'to' contextual; only 'lifetime'/'subscribe' are reserved; '<' lexed).

Analysis (ownlang/lifetimes.py, check_lifetimes):
- region escape OWN014: if SOURCE strictly outlives self, the strong capture
  promotes self to SOURCE's lifetime and it leaks. The *ordering* is what makes
  it a leak -- a same/shorter-lived source is clean. Per fork B the code is
  domain-neutral (OWN014 'escapes to a longer-lived region'), not WPF-branded.
- the mitigation (a disposable token released on close) is the slice-#1
  acquire/release pattern, so both halves of the theorem now exist.

Wiring/tests:
- CLI 'check' runs check_lifetimes; the new Subscribe stmt is a no-op for the
  loans/permissions flow and a schematic emit in codegen (both assert_never
  dispatchers updated; lower_let rhs got assert_never too).
- corpus/wpf/viewmodel-escapes-to-app (before/after C# + notes); test_wpf made
  tolerant of non-kinded cases. tests/test_lifetimes.py: 10 region cases.
- docs/lifetimes.md + README updated to mark slice #2 built.

Gate + suite green: mypy --strict, ruff, analysis 123/123, codegen 23/23, fuzz,
gallery 10/10, corpus 2/2, wpf 3/3, lifetimes 10/10.
PhysShell pushed a commit that referenced this pull request Jun 16, 2026
#1 --severity {error,warning} (default error): threaded through the core
renderer (Finding.render/_github/_msbuild + render_finding), the ownir CLI
(--severity, validated; only valid for ownir), own-check.sh, and the
composite Action's new 'severity' input. A presentation choice — the finding
is still the core's verdict — so a build can show leaks as advisory warnings
instead of failing. Resolves P-013 open question #1.

#2 scripts/own-check.ps1: a PowerShell twin of own-check.sh so Windows/VS
users without bash get the same one-command pipeline (same flags/output).

Docs: how-to §2B now uses --severity warning (drops the sed hack) and §4
points at own-check.ps1; P-013 scope/open-questions updated. CI: new step
asserts --severity warning emits warning-level (not error) MSBuild lines.

Verified locally: ownir 28/28, ruff + mypy --strict clean, severity honored
in github/msbuild/human, bad/misapplied severity returns 2.

https://claude.ai/code/session_01BE8MeHTtFcrTBjtMAQ7Yc6
PhysShell added a commit that referenced this pull request Jun 16, 2026
…egy-gs41zk

level-2: add --severity flag + PowerShell own-check.ps1

#1 --severity {error,warning} (default error): threaded through the core
renderer (Finding.render/_github/_msbuild + render_finding), the ownir CLI
(--severity, validated; only valid for ownir), own-check.sh, and the
composite Action's new 'severity' input. A presentation choice — the finding
is still the core's verdict — so a build can show leaks as advisory warnings
instead of failing. Resolves P-013 open question #1.

#2 scripts/own-check.ps1: a PowerShell twin of own-check.sh so Windows/VS
users without bash get the same one-command pipeline (same flags/output).
@coderabbitai coderabbitai Bot mentioned this pull request Jun 18, 2026
PhysShell pushed a commit that referenced this pull request Jun 18, 2026
… re-validation

Leak #4 in the SystemEvents fixture: a FileStream that IS disposed, but the
Dispose() sits inside the try after a may-throw call (WriteByte), so it's
skipped on the exceptional path. This is CodeQL's cs/dispose-not-called-on-throw
(cs/local-not-disposed also models it). Own.NET used to miss it — disposed
somewhere looked balanced — until the exception-edge model (77b2edd) inserted a
throw edge before each may-throw statement in a try.

Re-running the oracle on this fixture should land #4 in "Agree" with CodeQL,
joining #2/#3, with #1 (subscription) staying Own.NET-only. Bumps the oracle
sentinel to trigger the push-run.
PhysShell pushed a commit that referenced this pull request Jun 18, 2026
…oracle fixture

The exception-edge try-lowering injects an exceptional exit (a bare return while
the local is live) before each may-throw statement. A local never disposed then
leaks on BOTH that exit and the normal fall-through, so the core emits OWN001
once per exit. Every flow-local diagnostic remaps to the acquire line, so the two
collapse to byte-identical findings. The first oracle run surfaced this as
Program.cs:54 appearing twice in "Agree".

- ownir.py: drop byte-identical findings (same file/line/code/component/event/
  handler/message/kind/severity) before sorting. Native-OwnLang leaks at distinct
  lines stay distinct — the key includes line; only the bridge's line-collapse
  makes them identical, so deduping is exactly right.
- test_ownir.py + flow_leak_two_exits fixture: a two-leaking-exits flow body
  (acquire; if(*){return}; use) must yield exactly one OWN001 (TryNeverDisposed
  'tfLeak'@105). Without the dedup it returns two.

Also tighten the SystemEvents fixture's dispose-on-throw case (#4): the three
tools anchor the same leak at different points (Own.NET acquire / CodeQL Dispose /
Infer# last-access), so a spread-out method puts them >3 lines apart and the
oracle's ±3 window splits one leak into own-only + oracle-only. Keep the try a
one-liner adjacent to the acquire so the anchors fall inside the window -> #4
joins #2/#3 in "Agree" across all three. Bumps the oracle sentinel.
PhysShell added a commit that referenced this pull request Jun 22, 2026
…tion

fix(extractor): null-conditional dispose + cross-member pool release (mined ImageSharp FP #2,#3/4)
PhysShell pushed a commit that referenced this pull request Jun 23, 2026
…t rebinds (Codex P2 + CodeRabbit)

The first cut keyed the alias map by identifier STRING, scanned class-wide, and
gated reassignment only on `=` expressions. Three holes, all flagged in review:

- CodeRabbit (major) + Codex #2 — class-wide name keys conflate same-named locals
  across methods: an unrelated `c.Dispose()` in one method could credit a field
  aliased by a `c` in another (FN), and two aliases reusing a name overwrite each
  other. Now keyed by the LOCAL SYMBOL (model.GetDeclaredSymbol(decl)), and the
  dispose scan resolves the receiver symbol — so each alias is scope-exact.
- Codex #1 — an alias rebound through a `ref`/`out` argument (`Swap(ref a)`) was
  not counted as reassigned, so a disposed-after-rebind alias wrongly credited the
  field (FN). The reassignment set now includes ref/out argument locals.
- Iterating declarators by symbol (is ILocalSymbol) also drops non-local
  declarators from the map (locals-only, as intended).

Direct field disposals are unchanged: a receiver that is not an alias local falls
back to FieldName, exactly as before.

Regression controls added: RefReboundAliasLeaks (`Swap(ref a)` then dispose ->
_refRebound still leaks) and ScopedAliasNameLeak (an unrelated same-named `c`
disposed in another method must not credit _scopedLeak -> it still leaks). CI
asserts both, alongside the existing aliased-silent and never-disposed/`=`-rebound
controls.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
PhysShell pushed a commit that referenced this pull request Jun 28, 2026
…+ no-op-dispose BCL wrappers

Two precision fixes for the remaining open root-causes in oracle-known-fps.md.

Tail 1 (root-cause #1, custom dispose-sink): release a tracked local used as
the resource of a `using (existingLocal) { ... }` statement. The --flow-locals
lowering already handled the `using (var x = ...)` declaration form and the
MemoryPool owner statement form, but not the expression form over an
already-acquired local (`var timer = new ...; using (timer) { ... }`) — it left
the local looking live at method exit, a spurious OWN001. Now threads a
scope-exit release onto the body's normal completion and its return/throw exits
(no acquire — the local was acquired at its `new`). Clears protobuf-net's
assorted/ Silverlight Page.xaml.cs `timer` (baseline entry marked fix-landed,
pending a live-oracle re-run to confirm-and-delete).

Tail 2 (root-cause #2, no-op Dispose): extend the StringWriter/StringReader
field exemption to BCL pass-through wrappers. A StreamReader/StreamWriter/
BinaryReader/BinaryWriter field whose every construction wraps an in-memory
backing (MemoryStream/StringWriter/StringReader) cascades disposal only to
managed memory, so it is dispose-optional. Closed allowlist of those four
adapters (GZipStream/CryptoStream own native/extra resource — stay flagged);
a path building `new StreamReader(path)` (a real handle) keeps the field
flagged. Does NOT clear Newtonsoft's _textWriter (a third-party JsonTextWriter
wrapper whose Dispose we can't prove is pass-through without body analysis —
the soundness wall); that stays baselined, with the reasoning recorded so it
isn't unsoundly suppressed.

Corpus fixtures: local-dispose-via-using-statement, field-noop-dispose-wrapper
(each: before.cs caught / after.cs clean, proving the exemption stays narrow).
Docs: new no-op-dispose-wrapper.md; oracle-known-fps.md root-causes #1/#2 and
baseline annotations updated. Selftests: oracle 31/31, corpus 29/29,
validate_contrib 16/16, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm
PhysShell pushed a commit that referenced this pull request Jun 30, 2026
… recursive Dispose-no-op idea

Investigated the deferred "recursive Dispose-is-a-no-op" feature against its only
motivating case (Newtonsoft TraceJsonReader._textWriter). Reading the real
JsonTextWriter source settles it: disposal is NOT a no-op type.

- Close() -> base.Close() auto-completes open JSON tokens (writes closing
  brackets to the underlying writer), then CloseBufferAndWriter().
- CloseBufferAndWriter() RETURNS the rented _writeBuffer to _arrayPool when an
  ArrayPool is configured — a real pooled-buffer release (the POOL-leak class
  Own.NET tracks) — and closes the underlying writer.

So a recursive Dispose-no-op recognizer would be unsound (it can leak a pooled
buffer) or would correctly decline — either way it does not clear this finding.
The _textWriter instance is benign only by instance facts (no ArrayPool set + the
sink is a StringWriter), not a type-level no-op — the same reason writers are
excluded from IsNoOpDisposeWrapper. Shelved as not worth building, not deferred.

Sharpened the baseline entry reason and updated no-op-dispose-wrapper.md +
oracle-known-fps.md root-cause #2 with the evidence. No code change. Selftests:
oracle 31/31, corpus 30/30.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm
PhysShell pushed a commit that referenced this pull request Jul 3, 2026
Both reviewers converged; all corrections are factual/design refinements
(no forks):

- Crate DAG: own-codegen is a SIBLING of own-diagnostics (both consume
  own-cfg/own-analysis), not chained through it. Codegen is
  verdict-independent (AST/CFG-driven, matching Python codegen.generate),
  so the old own-diagnostics -> own-codegen edge would have forced
  diagnostics to re-export solver internals. Added a fitness function
  locking codegen !-> diagnostics (and !-> analysis for now). (Codex P2,
  CodeRabbit #1)
- Oracle: do NOT reuse scripts/oracle_compare.py as the parity oracle —
  it is cross-tool fuzzy (leak-only, +-N line tolerance, coarse severity)
  and would mask off-by-one/label/subject/exit-status divergences. Spec a
  new exact harness over status+stdout+stderr+SARIF/JSON, with an
  exit/crash gate first, exact set-equality incl. evidence label text,
  and intra-tie ordering. (Codex P1, CodeRabbit #4)
- CFG seam does not exist yet: python cfg prints human text, not JSON.
  Add+freeze a canonical cfg --format json before the ratchet uses that
  seam; added as migration step 0. (Codex P2)
- State: arena+CoW likely wins this procedural workload; bench largest
  real function, wall-clock + RSS. Prior art: clippy lint-pass registry,
  prusti-viper encoding boundary. Repo-layout revisit trigger. (CodeRabbit
  #2/#3/#5/#6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants